-
Notifications
You must be signed in to change notification settings - Fork 147
Add support for custom scripts #1056
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is making an unintentional source breaking change that should be undone.
Additionally, we're in the process of migrating away from LocalFileSystemDataProvider
so the new code to find the custom-scripts.json file also need to be added to DocumentationContext.InputProvider
. There's already a test that verifies that both implementation discover the same files which would also be good to update so that it verifies the behavior for this new type of file.
Sources/SwiftDocC/Infrastructure/Workspace/LocalFileSystemDataProvider+BundleDiscovery.swift
Outdated
Show resolved
Hide resolved
@@ -120,6 +120,18 @@ struct ConvertFileWritingConsumer: ConvertOutputConsumer { | |||
for downloadAsset in context.registeredDownloadsAssets(forBundleID: bundleIdentifier) { | |||
try copyAsset(downloadAsset, to: downloadsDirectory) | |||
} | |||
|
|||
// Create custom scripts directory if needed. Do not append the bundle identifier. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not append the bundle ID here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- So that projects with combined documentation of multiple targets (when that lands) can share scripts between the documentation bundles of each target. I suspect that authors of packages with multiple targets will want scripts to be shared: nobody wants to set up their MathJax scripts once per target.
- In practice, even in documentation webpages for multiple targets, custom scripts loaded into the browser are global. For example, scripts that define the same global function can conflict with each other irrespective of the bundle they were loaded from. So namespacing the script files by bundle ID is moot.
(Also, if the relative path to the custom-scripts
folder is always "/custom-scripts"
then it’s easier to find it in the renderer code.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. That's not how combined documentation works (which is already available as an experimental feature). Non-content files aren't automatically included in the combined archive. If this was treated as an asset, a collision between archives would be treated as an error. If this file is intended to be copied over into the combined archive, it should have an archive-unique prefix and this PR should update the merge command to copy over the expected files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey David,
Sorry for the confusion; I hadn’t read your combined-documentation proposals yet and was operating under some admittedly hasty guesswork. Having read them now — the second, lower-level pitch in particular — I wasn't able to find more information on how combined documentation handles assets.
So I looked through the swift-docc
code. But I haven't yet been able to find code for merging documentation archives, or for handling assets during that merge (the closest I could find was this, which seems related to merging assets of documentation archives given the phrasing here), or for including additional "non-symbol content for the landing page of the combined documentation" (a feature of docc merge
you described in the technical pitch that could be relevant here). I'm definitely missing something obvious so apologies for the question, but where in the swift-docc
codebase does documentation-archive merging happen? Essentially, I'm trying to figure out:
- How documentation-archive merging handles the
theme-settings.json
files of the archives being combined, so I can do the same tocustom-scripts.json
. - How documentation-archive merging handles images, so I can do the same to the individual JS scripts in the archive's
custom-scripts
directory. (Assuming that doing so wouldn't break custom scripts, which I'll also look into.)
Thanks!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- How documentation-archive merging handles the
theme-settings.json
files of the archives being combined, so I can do the same tocustom-scripts.json
.
The combined archive is assumed to want a consistent visual appearance across all its modules. Therefore it doesn't matter which theme-settings.json file it uses (but it only uses one of them).
- How documentation-archive merging handles images, so I can do the same to the individual JS scripts in the archive's
custom-scripts
directory. (Assuming that doing so wouldn't break custom scripts, which I'll also look into.)
In a regular archive, images are already located in a subdirectory named after the documentation's identifier. This makes merging images trivial because each subdirectory can simply be copied into the combined archives "images" directory.
cb3a07b
to
b8512d2
Compare
Summary
First-class support for adding custom scripts to a DocC-generated website. These may be local scripts, in which case the website will continue to work offline, or they may be external scripts at a user-specified URL. This support is in the form of a custom-scripts.json file, the scripting analog of theme-settings.json.
Full pitch: https://forums.swift.org/t/pitch-support-for-custom-scripts-in-docc/75357.
Dependencies
Corresponding change in swift-docc-render.
Testing
custom-scripts.json
and the custommathjax-config.js
script (shown in the pitch) to your documentation catalog.docc convert
the documentation catalog with your custom scripts. Observe that the modified swift-docc copiedcustom-scripts.json
and the custom scripts into the documentation archive.Checklist
Make sure you check off the following items. If they cannot be completed, provide a reason.
./bin/test
script and it succeeded